Improve pull-to-refresh usability in RefreshContainer - #18617

You can test this PR using the following package version. 12.0.999-cibuild0055974-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

• All contributors have signed the CLA.

@alexander.marek,

Please read the following Contributor License Agreement (CLA). If you agree with the CLA, please reply with the following:

@cla-avalonia agree

Contributor License Agreement
1 out of 2 committers have signed the CLA.

• @nickodei
• @alexander.marek

alexander.marek doesn't seem to be a GitHub user. Please add the emails used in your commits to your Github account accordingly.

@cla-avalonia agree

This is actually mainly the code of #17496, with one addition:

Added a styled property "IsEnabledOnDesktop" that allows to optionally enable the pulltorefresh gesture on Desktop applications as well.

You can test this PR using the following package version. 12.0.999-cibuild0056035-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

Could you make your pr description more descriptive? Even if it's based on #17496 , it should still follow contribution guidelines.

@emmauss

Essentially, I

• cherry-picked all changes from Improve Refresh-Container usability #17496
• incorporated your feedback (the changes you requested in Improve Refresh-Container usability #17496)
• added the possibility, to also enable pull2refresh on desktops (mouse-controlled devices)

The third feature is important. It allows for

• easier testing of pull2refresh (mac and windows)
• opt in pull2refresh for customers, that are confused why "this works on my phone but not my computer" (yes, they exist...)

IsEnabledOnDesktop is a styled property
So it is disabled by default, but can easily be enabled using a global style.

Note that the propertyname is not of my concern.
I understand that you have API design meetings and will most likely change it. You've got my blessings to do so (although you do not need them ;-))

You can test this PR using the following package version. 12.0.999-cibuild0056624-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@emmauss

Essentially, I

* cherry-picked all changes from [Improve Refresh-Container usability #17496](https://github.com/AvaloniaUI/Avalonia/pull/17496)

* incorporated your feedback (the changes you requested in [Improve Refresh-Container usability #17496](https://github.com/AvaloniaUI/Avalonia/pull/17496))

* added the possibility, to also enable pull2refresh on desktops (mouse-controlled devices)

The third feature is important. It allows for

* easier testing of pull2refresh (mac and windows)

* opt in pull2refresh for customers, that are confused why "this works on my phone but not my computer" (yes, they exist...)

IsEnabledOnDesktop is a styled property So it is disabled by default, but can easily be enabled using a global style.

Note that the propertyname is not of my concern. I understand that you have API design meetings and will most likely change it. You've got my blessings to do so (although you do not need them ;-))

It would be best if you put this in the PR description and follow the section guides.

Public API for review:

 namespace Avalonia.Input.GestureRecognizers
 {
     public class ScrollGestureRecognizer : GestureRecognizer
     {
+        public static readonly DirectProperty<ScrollGestureRecognizer, Vector?> OffsetProperty;
+        public static readonly DirectProperty<ScrollGestureRecognizer, Size?> ExtentProperty;
+        public static readonly DirectProperty<ScrollGestureRecognizer, Size?> ViewportProperty;
+        public Size? Extent { get; private set; }
+        public Size? Offset { get; private set; }
+        public Size? Viewport { get; private set; }
     }
 }
 
 namespace Avalonia.Controls
 {
     public class RefreshContainer : ContentControl
     {
+        public static readonly StyledProperty<bool> IsEnabledOnDesktopProperty;
+        public bool IsEnabledOnDesktop { get; set; }
     }
 }

I personally think it needs a better name than IsEnabledOnDesktop. Since that implies it wont work at all on a desktop. Which from my understanding is not true, it just enables mouse controls. Perhaps a better name would be SupportAllPointerDevices or UseAllPointerDevices?

Alternatively, allowing customisation of the whitelist for what devices it accepts would also be better and more flexible. The current flag is kind of an all or touch only approach.

Thanks for the great development. Releasing this PR will finally fix my gesture orchestrating issues in android! Hopefully it happens soon

Notes from the API review:

Remove the properties added to ScrollGestureRecognizer

Exposing Extent, Offset and Viewport shouldn't be necessary. We don't want to add every possible scroll info property as part of the public API of the gesture recognizer.

Instead, the ScrollGestureRecognizer should listen to property change notifications from its parent presenter, which is accessible through the existing Target property.

Rename IsEnabledOnDesktop to IsMouseEnabled

The property isn't really related to desktop: it simply enables the mouse to be recognized, even on mobile platforms.

Notes from the API review:

Remove the properties added to ScrollGestureRecognizer

Exposing Extent, Offset and Viewport shouldn't be necessary. We don't want to add every possible scroll info property as part of the public API of the gesture recognizer.

Instead, the ScrollGestureRecognizer should listen to property change notifications from its parent presenter, which is accessible through the existing Target property.

Rename IsEnabledOnDesktop to IsMouseEnabled

The property isn't really related to desktop: it simply enables the mouse to be recognized, even on mobile platforms.

Could you please give me a hint on how to do this?

Because IScrollable and ScrollViewer are actually defined in Avalonia.Controls which I do not have a reference to.

So simply deleting those properties and "using those of the Target" is not possible:

The Renaming has been done ;-)

Because IScrollable and ScrollViewer are actually defined in Avalonia.Controls which I do not have a reference to.

Sorry for the late reply, you're completely right.

I'm not sure why ScrollGestureRecognizer is in the Base assembly when it's only used in Controls but that ship has sailed.

You can test this PR using the following package version. 12.0.999-cibuild0057964-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

You can test this PR using the following package version. 12.0.999-cibuild0058042-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

I reworked the recognizer - at some points the pull to refresh gesture had no more effect.
Also added test.

You can test this PR using the following package version. 12.1.999-cibuild0065280-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

You can test this PR using the following package version. 12.1.999-cibuild0066439-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

You can test this PR using the following package version. 12.1.999-cibuild0066720-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

Note: I've updated the tests to avoid reflection.